Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

2nd draft for Registering an Image #2784

Merged
merged 1 commit into from
Jan 31, 2022
Merged

2nd draft for Registering an Image #2784

merged 1 commit into from
Jan 31, 2022

Conversation

TDDR
Copy link
Contributor

@TDDR TDDR commented Jan 31, 2022 •
edited
Loading

Issue This PR Addresses

Fixes #1743

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

This is my second draft of this after attempting to use a different workflow in a previous iteration. I have tried multiple different ways to try and get the buider to go into a sub directory instead of using the Dockerfile in the root with varying results
image

I think the version in this PR is the proper way to go about it, but it's not supported yet
image
image

I could be wrong but I'm not sure what else to try.

@TDDR TDDR self-assigned this Jan 31, 2022
@gitpod-io
Copy link

gitpod-io bot commented Jan 31, 2022

@TDDR TDDR changed the title Draft: 2nd draft Draft: 2nd draft for Registering an Image Jan 31, 2022
@TDDR TDDR added area: deployment Production or Staging deployment area: docker area: microservices type: enhancement New feature or request labels Jan 31, 2022
@TDDR TDDR marked this pull request as draft January 31, 2022 15:47
@TDDR TDDR changed the title Draft: 2nd draft for Registering an Image 2nd draft for Registering an Image Jan 31, 2022
@TDDR TDDR added the help wanted Extra attention is needed label Jan 31, 2022
@TDDR TDDR added this to the 2.6 Release milestone Jan 31, 2022
cindyorangis
cindyorangis reviewed Jan 31, 2022
View reviewed changes
.github/workflows/deploy-feed-discovery-image.yaml Outdated
on:
push:
branches:
- 'issue-1743'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very interested in this GitHub Container Registry stuff. Were you able to push an image from your personal issue-1743 branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet, but having that allows the workflow to trigger when I push it to my fork for testing purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple if the iterations went through the entire process but failed on the final push, but it was using the wrong Dokerfile anyway. I figure if I can get it to do this on the proper file then when I switch that to master and merge it it should work
image

humphd
humphd requested changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great. One more thing I notice.

.github/workflows/deploy-feed-discovery-image.yaml Outdated Show resolved Hide resolved
@TDDR TDDR marked this pull request as ready for review January 31, 2022 16:46
humphd
humphd previously approved these changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try this and see what it does.

NOTE: we'll need to make a change within src/api/feed-discovery/** for this to do anything once it lands. Maybe update one of the deps?

@TDDR TDDR removed the help wanted Extra attention is needed label Jan 31, 2022
@humphd
Copy link
Contributor

humphd commented Jan 31, 2022

@TDDR can you squash, rebase, and reword this?

git checkout master
git pull upstream master
git checkout issue-1743
git rebase -i master
...change first commit to reword, second to fixup, save, then rewrite commit message, save again
git push origin issue-1743 -f

manekenpix
manekenpix previously approved these changes Jan 31, 2022
View reviewed changes
cindyorangis
cindyorangis previously approved these changes Jan 31, 2022
View reviewed changes
@TDDR TDDR dismissed stale reviews from cindyorangis, manekenpix, and humphd via ba00920 January 31, 2022 17:18
humphd
humphd previously approved these changes Jan 31, 2022
View reviewed changes
@TDDR TDDR requested a review from humphd January 31, 2022 18:53
@TDDR TDDR merged commit b5750a9 into Seneca-CDOT:master Jan 31, 2022
@TDDR TDDR mentioned this pull request Jan 31, 2022
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@humphd humphd humphd approved these changes

@TueeNguyen TueeNguyen TueeNguyen approved these changes

@aserputov aserputov aserputov approved these changes

@sirinoks sirinoks Awaiting requested review from sirinoks

@raygervais raygervais Awaiting requested review from raygervais

@manekenpix manekenpix Awaiting requested review from manekenpix

@cindyorangis cindyorangis Awaiting requested review from cindyorangis

Assignees

@TDDR TDDR

Labels
area: deployment Production or Staging deployment area: docker area: microservices type: enhancement New feature or request
Projects
None yet
Milestone
2.6 Release
Development

Successfully merging this pull request may close these issues.

Automate Docker build of Feed-Discovery service with GitHub Actions
6 participants
@TDDR @humphd @manekenpix @cindyorangis @TueeNguyen @aserputov